[AgentX] vLLM DeepSeek-V4 B200 aggregate / vLLM DeepSeek-V4 B200 聚合配置#2224
[AgentX] vLLM DeepSeek-V4 B200 aggregate / vLLM DeepSeek-V4 B200 聚合配置#2224cquil11 wants to merge 3 commits into
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 如需进行 PR 验证,请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
1 similar comment
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 如需进行 PR 验证,请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29432192113 |
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
| set -eo pipefail | |||
There was a problem hiding this comment.
🟡 Line 2 changed set -euo pipefail to set -eo pipefail, dropping nounset — this is the only script among all 24 in benchmarks/single_node/agentic/ that doesn't use -u. The rewrite replaced every ${VAR:-default}-guarded reference ($DCP_SIZE, $SLURM_JOB_ID, $SLURMD_NODENAME, $MODEL_PATH, $KV_OFFLOAD_BACKEND) with bare $VAR forms, which is why -u had to go; the near-identical dsv4_fp4_b300_vllm.sh shows the same optional-var pattern works fine with -u kept and ${VAR:-} guards. Recommend restoring set -euo pipefail and guarding the new bare references, to keep this script's safety net consistent with its siblings.
Extended reasoning...
The bug: Line 2 of dsv4_fp4_b200_vllm.sh was changed from set -euo pipefail to set -eo pipefail, silently dropping the nounset (-u) flag. Every other script in benchmarks/single_node/agentic/ — including the near-identical dsv4_fp4_b300_vllm.sh, plus dsv4_fp4_mi355x_vllm.sh, dsv4_fp4_b200_sglang.sh, dsv4_fp8_h200.sh, and all the kimik2.5_*/qwen3.5_*/minimaxm3_* variants — still uses set -euo pipefail. This script is now the sole outlier in the directory.\n\nWhy it happened: The diff rewrote several previously-guarded optional-variable references into bare, unguarded forms: DCP_SIZE=${DCP_SIZE:-1} became an if [ -z "$DCP_SIZE" ]; then DCP_SIZE=1; fi block (which still reads the bare $DCP_SIZE first), ${SLURM_JOB_ID:-}/${SLURMD_NODENAME:-unknown} became bare $SLURM_JOB_ID/$SLURMD_NODENAME, ${MODEL_PATH:-} became bare $MODEL_PATH, and the new case "$KV_OFFLOAD_BACKEND" in "") branch reads $KV_OFFLOAD_BACKEND unguarded (note it isn't even part of check_env_vars, so it can legitimately be unset). None of these would survive set -u if the variable is genuinely unset, so -u had to be dropped to make the rewritten code work as written.\n\nWhy this is avoidable: The sibling dsv4_fp4_b300_vllm.sh proves -u and this exact optional-variable pattern coexist fine: it keeps set -euo pipefail while using DCP_SIZE="${DCP_SIZE:-1}", declare -p SLURM_JOB_ID >/dev/null 2>&1 && [ -n "$SLURM_JOB_ID" ], and similar guards for MODEL_PATH. The new case statement here could equally have matched on "${KV_OFFLOAD_BACKEND:-}" instead of the bare variable. So dropping -u wasn't required by the case-statement refactor — it was a side effect of un-guarding several other references at the same time.\n\nImpact: With -u disabled, a future typo in a variable name anywhere in this script (e.g. RESULT_DIR misspelled as RESUT_DIR) will silently expand to an empty string and continue executing rather than aborting immediately with an "unbound variable" error — exactly the failure mode -u exists to catch. As written today the script runs correctly (all currently-referenced optional vars are handled, and required vars are separately validated by check_env_vars), so there's no concrete runtime failure right now. This is a latent robustness/consistency regression rather than an active bug.\n\nProof by example: Suppose a future edit adds echo "Writing to $RESUT_DIR" (a typo of $RESULT_DIR) somewhere in this script. With set -eo pipefail (current state), $RESUT_DIR expands to "", the echo prints "Writing to ", and the script continues — the typo goes unnoticed until output is inspected downstream. With set -euo pipefail (every sibling script's setting), the same line would immediately abort with RESUT_DIR: unbound variable, catching the bug at the point of introduction. This is precisely why the convention exists across all 23 other scripts in the directory.\n\nFix: Restore set -euo pipefail on line 2 and guard the newly-unguarded references the same way dsv4_fp4_b300_vllm.sh does: DCP_SIZE="${DCP_SIZE:-1}"/PCP_SIZE="${PCP_SIZE:-1}", [[ -n "${SLURM_JOB_ID:-}" ]] / ${SLURMD_NODENAME:-unknown}, [[ -n "${MODEL_PATH:-}" ]], and case "${KV_OFFLOAD_BACKEND:-}" in.
53cf054 to
455eabc
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29432414334 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29445892486 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29445892486 |
|
vLLM recipe PR: vllm-project/recipes#645 |
Summary
origin/main.Original PR: #2202
Earlier source PR referenced by #2202: #2188
Testing
bash -n benchmarks/single_node/agentic/dsv4_fp4_b200_vllm.shpython -m pytest utils/matrix_logic/ -q— 221 passed.中文说明
origin/main。原 PR:#2202
#2202 引用的更早来源 PR:#2188
测试
bash -n benchmarks/single_node/agentic/dsv4_fp4_b200_vllm.shpython -m pytest utils/matrix_logic/ -q— 221 项测试通过。